nit: Is this required? Would be good to keep this as simple as possible. #Closed

Yes, It's required.

require_static_shape_ or not will determinate the what the sub-graph looks like. And gen_metadef_name didn't check the input/output shape info.

We should document what we do and don't do here.

• Prefer cache key from model metadata
  • we could include example python here, and skip the actual implementation of the hashing to keep it simple
• Use model path if available

If model changes user must do one of:

• Set different cache key in model metadata
• Load model from a different path
• Delete old cache information #Closed

Or we can document fully in CoreML-ExecutionProvider.md and include a link to that here.

Sure.

We should have a const for the model metadata key name here. I'd vote for COREML_CACHE_KEY given the usage is CoreML specific. #Closed

Sure.
May the cache_key can be used by other EPs.

Given the cache is an optional feature, it might be better to disable caching and log the error instead of throwing, as it could break an iOS app completely (e.g. if they don't have logic to explicitly turn off caching it will be broken). #Closed

This won't be possible to throw as long as CoreML EP developers didn't modify gen_metadef_name or modify both.

User can unset 'coreml_options.ModelCachePath()' to disable cache.

But it should be fine to remove this check.

This is only true if the model path is available. If not it hashes the graph input names and all the node output names. #Closed

I have documented it in CoreML EP.
I will add more comments here.

We need to validate this with something like std::isalnum to guarantee it will be valid for use in the filesystem. I'd also suggest we enforce a maximum length to also try and avoid issues creating folders/files with this in the name. #Closed

Yeah. we should do it.
For now, cache-key should has at most 32 characters. Or we will re-hash it.

64 chars + null is required to store a sha256 hash as hex so maybe we could use that as the limit?

64 comes to my mind in the first thought, would this be too long?
Let me use 64 as the limit again.

it's a maximum so the user isn't forced to use it all. Allowing a 256 bit hash seems reasonable. Max directory/file name limit is 255 chars so 64 is well under that. Max path is somewhere around 1000 chars. User also controls the cache path.

But we should be careful about how deep the directory names get for the cache files. It might make more sense to shorten things like whether static shapes were enabled, and for the model it's really only the subgraph id that matters.

i.e. including the user hash and the model hash in a directory name (userhash_COREML_modelhash_subgraphid) shouldn't actually matter when all files for the model are stored under a top-level directory name that uses the preferred hash, as there a) should never be files from any other models in there, and b) the subgraphid should be deterministic (unless they run with a different optimization level).

Take a look at how long the paths are in your tests and figure out a good balance between readable/safe and avoiding exceeding the max path length.

Sure. It's quite balanced now.
<user_provided_path>/cc6b2111b15dcdcf00ed6647c4430315e01616efde46ab4109f80fd1d3c46731/0_dynamic_mlprogram/model/